Skip to content

tests: fix static link order#478

Merged
wprzytula merged 1 commit into
scylladb:masterfrom
wprzytula:tests-fix-openssl-in-static-linkage
Jun 15, 2026
Merged

tests: fix static link order#478
wprzytula merged 1 commit into
scylladb:masterfrom
wprzytula:tests-fix-openssl-in-static-linkage

Conversation

@wprzytula

@wprzytula wprzytula commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What's done

Place the driver target before CASS_LIBS in target_link_libraries for integration tests. With static linking the linker resolves symbols left-to-right, so the archive providing symbols must precede the libraries that define them.

It's fascinating that the bug comes from the DataStax CPP Driver's build system. They must have never tested static linkage with integration tests.

Verification

cmake\
-DCASS_BUILD_INTEGRATION_TESTS=ON \
-DCASS_USE_STATIC_LIBS=ON \
-B build \
&& (cd build && make)

now works. Before, it would fails with undefined references to OpenSSL symbols.

Further work

#438 apart of containing this fix, introduced a lot of changes and static linkage verification using the smoke test app. I'm working on making the changes more granular, better documented, and hence mergable.

Fixes: #164

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • PR description sums up the changes and reasons why they should be introduced.
  • [ ] I have implemented Rust unit tests for the features/changes introduced.
  • [ ] I have enabled appropriate tests in Makefile in {SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER.
  • I added appropriate Fixes: annotations to PR description.

@wprzytula @dkropachev
tests: fix static link order
2a9ca24 
Place the driver target before CASS_LIBS in target_link_libraries
for integration tests. With static linking the linker resolves
symbols left-to-right, so the archive providing symbols must
precede the libraries that define them.

```sh
cmake\
-DCASS_BUILD_INTEGRATION_TESTS=ON \
-DCASS_USE_STATIC_LIBS=ON \
-B build \
&& (cd build && make)
```
now works. Before, it would fails with undefined references to OpenSSL
symbols.

Fixes: scylladb#164

Co-authored-by: Dmitry Kropachev <dmitry.kropachev@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f2530409-546b-4bd7-8801-2ba554a4d30f

📥 Commits

Reviewing files that changed from the base of the PR and between ead94af and 2a9ca24.

📒 Files selected for processing (1)
  • tests/src/integration/CMakeLists.txt
📝 Walkthrough

Walkthrough

In tests/src/integration/CMakeLists.txt, the target_link_libraries call for the cassandra-integration-tests target has its two entries reordered: ${PROJECT_LIB_NAME_TARGET} is now listed before ${CASS_LIBS}, reversing the previous order. This affects how the linker resolves symbols when building integration tests with static libraries (e.g., under -DCASS_USE_STATIC_LIBS=ON).

Possibly related PRs

  • scylladb/cpp-rs-driver#473: Removes duplicate exported test stub symbols from src/testing_unimplemented.cpp, targeting the same static-linking symbol-resolution issue as this reorder.

Suggested labels

area/testing

Suggested reviewers

  • Lorak-mmk
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'tests: fix static link order' directly and accurately describes the main change: reordering linker arguments in the test CMakeLists to fix static linking.
Description check ✅ Passed The PR description covers what's done, verification steps, and fixes annotations. Most pre-review checklist items are completed with appropriate strikethrough for non-applicable items.
Linked Issues check ✅ Passed The code change directly addresses issue #164 by reordering linker arguments to resolve undefined OpenSSL symbol references when building with static linking and integration tests enabled.
Out of Scope Changes check ✅ Passed The single file change in CMakeLists.txt is narrowly focused on reordering linker arguments to fix the static linkage issue described in issue #164, with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@wprzytula wprzytula self-assigned this Jun 15, 2026
@wprzytula wprzytula requested review from Lorak-mmk and Copilot June 15, 2026 10:02
@wprzytula wprzytula added the area/testing Related to unit/integration testing label Jun 15, 2026
@wprzytula wprzytula added this to the 1.1.0 milestone Jun 15, 2026
Copilot AI reviewed Jun 15, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes static-link integration test build failures by correcting the library link order for cassandra-integration-tests so that the driver archive appears before its transitive dependencies (notably OpenSSL), matching how other executables in the repo are linked.

Changes:

  • Reorder target_link_libraries() entries so ${PROJECT_LIB_NAME_TARGET} precedes ${CASS_LIBS} for the integration test executable.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wprzytula wprzytula merged commit 2a88ed3 into scylladb:master Jun 15, 2026
17 of 18 checks passed
@wprzytula wprzytula deleted the tests-fix-openssl-in-static-linkage branch June 15, 2026 13:02
@coderabbitai coderabbitai Bot mentioned this pull request Jun 16, 2026
Merged
4 tasks
@wprzytula wprzytula mentioned this pull request Jun 16, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@Lorak-mmk Lorak-mmk Lorak-mmk approved these changes

Assignees

@wprzytula wprzytula

Labels

area/testing Related to unit/integration testing

Projects

None yet

Milestone

1.1.0

Development

Successfully merging this pull request may close these issues.

Make -DCASS_BUILD_INTEGRATION_TESTS=ON -DCASS_USE_STATIC_LIBS=ON work

3 participants

@wprzytula @Lorak-mmk